Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(HMS-2994): delete domain confirmation #49

Merged
merged 1 commit into from
Apr 26, 2024

Conversation

avisiedo
Copy link
Contributor

@avisiedo avisiedo commented Apr 23, 2024

This change introduce the delete confirmation modal
dialog.

This dialog will show up when we click on the kebab
menu of the list domains view to delete the item, or
when we click on the kebab menu from the detail view
of the domain to delete the domain that is being
displayed.

In both cases, the deletion confirmation modal is
showed up, and when we press Delete button the
item is eventually deleted from the database.

Making click on the modal cross icon or cancel link
will dismiss the deletion confirmation modal with
no effect on the data stored.

@avisiedo avisiedo self-assigned this Apr 23, 2024
@avisiedo avisiedo force-pushed the hms-2994-delete-confirm branch 2 times, most recently from 7fc1f5e to 5201edf Compare April 24, 2024 15:40
Copy link
Contributor

@pvoborni pvoborni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works well. Mostly LGTM, but see the Q and comment.

@avisiedo avisiedo force-pushed the hms-2994-delete-confirm branch from 5201edf to 8d9df0b Compare April 25, 2024 09:50
@avisiedo avisiedo requested a review from pvoborni April 25, 2024 10:04
@avisiedo avisiedo force-pushed the hms-2994-delete-confirm branch from 8d9df0b to 7ab73e4 Compare April 25, 2024 13:17
@avisiedo
Copy link
Contributor Author

I have updated unit tests to be more accurate; the check for the text content of the modal was not working as I expected, so I have removed to avoid confusion; if some immediate way to add the check, happy to know about it.

@frasertweedale frasertweedale self-requested a review April 26, 2024 06:06
Copy link
Contributor

@pvoborni pvoborni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works. Code LGTM. Approving, but see the comment.

@avisiedo avisiedo force-pushed the hms-2994-delete-confirm branch from 7ab73e4 to c478299 Compare April 26, 2024 10:23
@avisiedo avisiedo requested a review from pvoborni April 26, 2024 10:23
@avisiedo avisiedo force-pushed the hms-2994-delete-confirm branch from c478299 to 4d656e0 Compare April 26, 2024 10:33
This change introduce the delete confirmation modal
dialog.

This dialog will show up when we click on the kebab
menu of the list domains view to delete the item, or
when we click on the kebab menu from the detail view
of the domain to delete the domain that is being
displayed.

In both cases, the deletion confirmation modal is
showed up, and when we press Delete button the
item is eventually deleted from the database.

Making click on the modal cross icon or cancel link
will dismiss the deletion confirmation modal with
no effect on the data stored.

Signed-off-by: Alejandro Visiedo <[email protected]>
@avisiedo avisiedo force-pushed the hms-2994-delete-confirm branch from 4d656e0 to 64c5ac3 Compare April 26, 2024 10:36
@avisiedo avisiedo merged commit d75ad47 into podengo-project:main Apr 26, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants